Fix library search matching interface control text#47
Conversation
|
Thanks for building Loop Library — the agent-native framing and the care in the catalog really show. This one's a small, self-contained fix. The homepage search built its haystack from each row's full I followed AGENTS.md — |
mberman84
left a comment
mberman84
left a comment
There was a problem hiding this comment.
The search-indexing change itself is correct and passed the integrated checks/browser verification. One deployment blocker remains: this changes site/script.js without changing the versioned script URL currently referenced as ?v=20260620-newest-first.
The live asset is cacheable for an hour with stale-while-revalidate, so existing visitors can continue receiving the old search behavior after deployment. Please bump the shared script asset version everywhere it is emitted and validated, including the generated loop-page template and scripts/check.mjs, then regenerate affected pages.
|
Thanks — good catch on the cache angle. Bumped the shared |
|
@mberman84 this is ready for another look whenever you have a moment — the asset version is bumped and the loop pages regenerated. Thanks for the careful review! |
mberman84
left a comment
mberman84
left a comment
There was a problem hiding this comment.
Correction to my immediately preceding review, whose inline code formatting was stripped by the shell:
The previously requested cache-bust fix is addressed, and the exact head passes node scripts/check.mjs, syntax checks, and all builders. The search-index change itself looks good.
This branch now needs a rebase onto current main before it can be approved. Since this PR branched, #61/#62 moved loop detail pages to the database-backed proxy and removed scripts/build-loop-pages.mjs plus site/loops/**. GitHub reports this head as conflicting, and merging the stale generated-page changes would risk restoring files the new architecture intentionally deleted.
Please rebase onto the latest main, keep the focused site/script.js search-index change, update the surviving shared script references/check assertions to a fresh version, and leave the removed generator/static loop pages deleted. Then rerun node scripts/check.mjs and the relevant asset checks.
mberman84
force-pushed
the
fix/search-excludes-control-text
branch
from
072d457 to
1c88752
Compare
mberman84
left a comment
mberman84
left a comment
There was a problem hiding this comment.
Rebased onto the database-backed main architecture. The focused search-index fix is preserved, stale generated-page artifacts are gone, and the full current CI-equivalent suite passes locally (41/41 Worker tests).
2 participants
Summary
The homepage library search matched interface control text, so several common words filtered nothing.
updateLibrary()built each row's search haystack from the entire row'stextContent:A loop row's
textContentincludes the "Copy loop" action button, and — afterscript.jsruns — the injected "Show more" / "Show less" prompt toggle. Because every row contains those strings, typingcopy,show,more, orlessmatched all loops instead of filtering them.Fix
Snapshot each row's searchable text once at load, reading the
.cell-loopcontent (category, attribution, title, summary, and prompt) before the prompt toggle button is inserted. The cached, pre-normalized value is then reused on every keystroke.This:
.cell-action"Copy loop" button and the injected toggle from the search index,data-searchkeyword matching fully intact, andtextContentfor all rows on every keystroke.Verification
copyorshow→ all 44 loops shown. After: those words match only loops whose content actually contains them.node scripts/check.mjs→Loop Library checks passed.node --check site/script.jsnode scripts/build-skill-catalog.mjs && node scripts/build-loop-pages.mjs && node scripts/build-social-images.mjs→ no artifact drift (git statusclean apart fromsite/script.js).npm --prefix worker run check→ tests pass.